Giza olympia fix bundled video creation and nft issuance by dobertRowneySr · Pull Request #3137 · Joystream/joystream

dobertRowneySr · 2022-01-31T05:58:31Z

fixes:

I also removed all the unused code like PersonId and the likes that was not removed this far, in order to have cleaner type aliases

Update 2022-02-02: rebased to current olympia . This lead to a larger number of commits
Relevant comits start from : c86b9dd
Relevant code section are in the content module, in particular:

NFTIssuanceParameter type introduced
InitTransactionalStatus type introduced
ensure_valid_init_transactional_status guard added
create_video & issue_nft extrinsic argument modified

vercel · 2022-01-31T05:58:36Z

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/joystream/pioneer-testnet/DrgnfQDUqymLYmQCctq2MRypMk1t
✅ Preview: Canceled

[Deployment for a6d433f canceled]

dobertRowneySr · 2022-02-02T18:06:04Z

The underlying issues look addressed. However, there are multiple issues with the code:

* no doc-comments for public types, extrinsics, and methods

* please, remove `allow all` clippy warning statement and fix all clippy issues in the pallet

* found this line in the code: `// TODO: Solve #now`

* rebase seems unsuccessful: 1) there is still unrelated code in the PR (removed empty extrinsics), 2) `generate_weights.sh` script fails now (incorrect benchmark name)

* there are public methods with no external usage (should be private)

the TODO was an old reference to myself that I solved and forgot to remove
benchmark should be fixed
removed the cargo directive
added doc-comments on types and helper functions added

I could locate the removed_empty_extrinsic reference in the Github commit tab

shamil-gadelshin · 2022-02-03T12:22:19Z

runtime-modules/content/src/lib.rs


            Self::deposit_event(RawEvent::VideoCategoryDeleted(actor, category_id));
        }
-


Isn't it a rebase artifact?

I am not following. The BASE commit in olympia has this code. In this PR that I am opening I removed it

Ok. So, it's part of the PR. I was expecting only the NFT-related code.

shamil-gadelshin

LGTM

vercel bot temporarily deployed to Preview January 31, 2022 05:58 Inactive

dobertRowneySr requested a review from shamil-gadelshin January 31, 2022 05:58

dobertRowneySr force-pushed the giza-olympia_fix_BundledVideoCreationAndNFTIssuance branch from a8043f2 to 0f174e1 Compare February 1, 2022 13:27

vercel bot temporarily deployed to Preview February 1, 2022 13:27 Inactive

vercel bot temporarily deployed to Preview February 1, 2022 14:26 Inactive

vercel bot temporarily deployed to Preview February 1, 2022 18:58 Inactive

ignazio added 23 commits February 2, 2022 14:59

added accept incoming offer tests

0d92708

added issuance type

384111f

added from implementation

82666f2

fixed mocks & buy_nft tests

e3b3152

fixed cancel_buy_now tests

115d810

fixed cancel nft auction tests

dd5e10f

fixed cancel offer tests

24a945f

cancel nft open auction bid

e7ff84c

updated claim won english auction tests

2fcca4a

Issue NFT tests updated

6e479bd

updated make bid tests

8bfa198

updated offer nft tests

ddbc791

updated open auction winner tests

4a17e30

updated sell nft tests

2f8cc37

updated sling back nft tests

bd0f724

updated start nft auction

8f6d3a5

small fix to tests

243a64a

updated accept incoming offer tests

3f3e0bf

updated fixed accept incoming offer tests

66ac939

added params validation before setting the status

ab72251

updated InitTransactionalStatus

cb2afbf

adedd checks for auction parameters

762ab90

added tests to ensure issue_nft init transactional status guard

6ff82e4

dobertRowneySr added 3 commits February 2, 2022 18:44

fixed test for the new TransactionalStatus alias

0752c9c

removed obsolete comment and added doc comments to nft/types

844b846

doc-commented helper functions

845e826

vercel bot temporarily deployed to Preview February 2, 2022 18:00 Inactive

dobertRowneySr requested a review from shamil-gadelshin February 2, 2022 18:07

vercel bot temporarily deployed to Preview February 3, 2022 10:22 Inactive

dobertRowneySr force-pushed the giza-olympia_fix_BundledVideoCreationAndNFTIssuance branch from 5884162 to 89376b3 Compare February 3, 2022 10:50

vercel bot temporarily deployed to Preview February 3, 2022 10:50 Inactive

dobertRowneySr force-pushed the giza-olympia_fix_BundledVideoCreationAndNFTIssuance branch from 89376b3 to af45c7f Compare February 3, 2022 10:51

vercel bot temporarily deployed to Preview February 3, 2022 10:51 Inactive

dobertRowneySr force-pushed the giza-olympia_fix_BundledVideoCreationAndNFTIssuance branch from af45c7f to fe745c8 Compare February 3, 2022 10:52

vercel bot temporarily deployed to Preview February 3, 2022 10:52 Inactive

shamil-gadelshin reviewed Feb 3, 2022

View reviewed changes

fixed format errors

195458d

dobertRowneySr force-pushed the giza-olympia_fix_BundledVideoCreationAndNFTIssuance branch from fe745c8 to 195458d Compare February 3, 2022 12:32

vercel bot temporarily deployed to Preview February 3, 2022 12:32 Inactive

clippy warnings fixed

30f8eaf

vercel bot temporarily deployed to Preview February 3, 2022 13:14 Inactive

This was referenced Feb 3, 2022

Content no unused code #2680

Closed

Giza olympia fix nft init state and issuance #3133

Closed

dobertRowneySr requested a review from shamil-gadelshin February 3, 2022 15:00

updated types

eb66efd

vercel bot temporarily deployed to Preview February 4, 2022 12:38 Inactive

generated types

a6d433f

dobertRowneySr force-pushed the giza-olympia_fix_BundledVideoCreationAndNFTIssuance branch from cfd1513 to a6d433f Compare February 4, 2022 12:44

vercel bot temporarily deployed to Preview February 4, 2022 12:44 Inactive

shamil-gadelshin approved these changes Feb 4, 2022

View reviewed changes

shamil-gadelshin merged commit bf0d7c7 into Joystream:olympia Feb 4, 2022

mnaamani mentioned this pull request Feb 5, 2022

Olympia: Fix QueryNode build #3174

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Giza olympia fix bundled video creation and nft issuance#3137

Giza olympia fix bundled video creation and nft issuance#3137
shamil-gadelshin merged 42 commits intoJoystream:olympiafrom
dobertRowneySr:giza-olympia_fix_BundledVideoCreationAndNFTIssuance

dobertRowneySr commented Jan 31, 2022 •

edited

Loading

Uh oh!

vercel bot commented Jan 31, 2022 •

edited

Loading

Uh oh!

dobertRowneySr commented Feb 2, 2022 •

edited

Loading

Uh oh!

shamil-gadelshin Feb 3, 2022

Uh oh!

dobertRowneySr Feb 3, 2022 •

edited

Loading

Uh oh!

shamil-gadelshin Feb 3, 2022

Uh oh!

shamil-gadelshin left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants


		Self::deposit_event(RawEvent::VideoCategoryDeleted(actor, category_id));
		}

Conversation

dobertRowneySr commented Jan 31, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vercel bot commented Jan 31, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dobertRowneySr commented Feb 2, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

shamil-gadelshin Feb 3, 2022

Choose a reason for hiding this comment

Uh oh!

dobertRowneySr Feb 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shamil-gadelshin Feb 3, 2022

Choose a reason for hiding this comment

Uh oh!

shamil-gadelshin left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

dobertRowneySr commented Jan 31, 2022 •

edited

Loading

vercel bot commented Jan 31, 2022 •

edited

Loading

dobertRowneySr commented Feb 2, 2022 •

edited

Loading

dobertRowneySr Feb 3, 2022 •

edited

Loading